-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#641 Added tooling tests #880
Conversation
@@ -56,7 +56,7 @@ class ArchUnitTestDescriptor extends AbstractArchUnitTestDescriptor implements C | |||
private ClassCache classCache; | |||
|
|||
private ArchUnitTestDescriptor(ElementResolver resolver, Class<?> testClass, ClassCache classCache) { | |||
super(resolver.getUniqueId(), testClass.getSimpleName(), ClassSource.from(testClass), testClass); | |||
super(resolver.getUniqueId(), testClass.getName(), ClassSource.from(testClass), testClass); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this because Jupiter would group field-flavored and method-flavored test results under different parent nodes (one under FQ class name and the other under simple class name)
ext.minimumJavaVersion = JavaVersion.VERSION_1_9 | ||
|
||
sourceSets { | ||
testFixtures { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't just apply java-test-fixtures
because then the source set gets added to the classpath in the form of a Jar. This messes up the test engines, which depend on file paths inside the resources directories
} | ||
} | ||
|
||
task removeFromMavenLocal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This task cleans the local Maven repo to make sure we are running the Surefire and Gradle tests with an up-to-date version of ArchUnit jars
archunit-tooling-test/build.gradle
Outdated
} | ||
|
||
gradle.taskGraph.whenReady { graph -> | ||
if (graph.hasTask(publishJUnitDepsToMavenLocal)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed because all the publishMavenJavaPublicationToMavenLocal
tasks depend on test
in their respective subprojects, so basically, publishJUnitDepsToMavenLocal
would re-trigger the tests for all of those subprojects that we need in the local Maven repo.
Perhaps there's a better way of doing the same, didn't really investigate
|
||
@Override | ||
protected String getBuildToolWrapperDirectory() { | ||
return "gradle"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Gradle wrapper is not really used for now (note the explicit Gradle version specified inside GradleEngine
)
Not sure which approach is better: to rely on wrappers inside testFixtures/resources
, or to keep Gradle/Maven versions inside the test framework code (the former is arguably faster, since Gradle/Maven doesn't need to be re-downloaded each time)
import java.util.Map; | ||
import java.util.stream.Stream; | ||
|
||
public class BaselineTest extends BaseTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test isn't testing ArchUnit as such, it is only here to test the different test engines themselves (as a sort of sanity check).
It was useful during the test framework development - not sure if we want to keep it or not. Might still be useful if new test engines are introduced (e.g. for IDEs)
|
||
// Dependencies for tool integrations | ||
surefireApi : [group: 'org.apache.maven.surefire', name: 'surefire-api', version: '3.0.0-M7'], | ||
gradleApi : [group: 'dev.gradleplugins', name: 'gradle-api', version: '7.4.1'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gradleApi
and byteBuddy
are not yet used anywhere (they will be used for Gradle filtering)
Signed-off-by: Krzysztof Sierszeń <krzysztof.sierszen@digitalnewagency.com>
8b0a3ad
to
604c899
Compare
Hey, thanks for all the hard work, but can you please put in a detailed description of why (I just see no description provided)? My problem here is: This adds 2.5K lines of code that we will have to maintain in the future. And I don't exactly see the benefit yet. In particular, what would be important to me is to weigh this against the simple system property filter suggested in the issue and then give detailed reasons why not to go the simple way. |
Signed-off-by: Krzysztof Sierszeń <krzysztof.sierszen@digitalnewagency.com>
Signed-off-by: Krzysztof Sierszeń <krzysztof.sierszen@digitalnewagency.com>
I implemented a simplified version in #1280. Since I have no capacity to work on this further, I'm gonna close this. Feel free to reopen if you want to pick this up or add further improvements. |
No description provided.